From e6c2f6c26d5dfea5896e9d0e8b9052d60b7a7fde Mon Sep 17 00:00:00 2001 From: tsteven4 <13596209+tsteven4@users.noreply.github.com> Date: Mon, 27 Jan 2020 12:22:23 -0700 Subject: [PATCH] modify format access to waypts via global list. (#478) As a rule writers should not modify waypoints on the global list. One way this can happen is if the writer uses the global waypoint list, global_waypoint_list. Via the list pointers to non-const Waypoints can be accessed. The three writers that use the global_waypoint_list all did not modify any waypoints, but it was not obvious. This PR makes it obvious by getting const Waypoint pointers from the list. This required some const correctness corrections in these writers. As a rule readers should not modify waypoints they didn't add to the global list. One way this can happen is if the reader uses the global waypoint list. One reader was inapproprieately modifying waypoints on the global list. While this is OK for waypoints it added, it is not OK for any waypoints that were added by other readers. This PR fixes a bug where the netsumbler reader, intending to modify waypoints it added to the list, actually modified all waypoints on the list. --- garmin.cc | 4 ++-- lowranceusr.cc | 14 +++++++------- netstumbler.cc | 21 ++++++++++++--------- tomtom.cc | 6 +++--- 4 files changed, 24 insertions(+), 21 deletions(-) diff --git a/garmin.cc b/garmin.cc index a49bdef25..669c94496 100644 --- a/garmin.cc +++ b/garmin.cc @@ -890,7 +890,7 @@ waypt_write_cb(GPS_PWay*) * description. */ static const char* -get_gc_info(Waypoint* wpt) +get_gc_info(const Waypoint* wpt) { if (global_opts.smart_names) { if (wpt->gc_data->type == gt_virtual) { @@ -935,7 +935,7 @@ waypoint_prepare() i = 0; // Iterate with waypt_disp_all? - foreach(Waypoint* wpt, *global_waypoint_list) { + for (const Waypoint* wpt : qAsConst(*global_waypoint_list)) { char obuf[256]; QString src; diff --git a/lowranceusr.cc b/lowranceusr.cc index 6435b5864..46481bcd9 100644 --- a/lowranceusr.cc +++ b/lowranceusr.cc @@ -472,13 +472,13 @@ register_waypt(const Waypoint* ref) /* end borrowed from raymarine.c */ -static Waypoint* +static const Waypoint* lowranceusr4_find_waypt(uint uid_unit, int uid_seq_low, int uid_seq_high) { lowranceusr4_fsdata* fs = nullptr; // Iterate with waypt_disp_all? - foreach (Waypoint* waypointp, *global_waypoint_list) { + for (const Waypoint* waypointp : qAsConst(*global_waypoint_list)) { fs = (lowranceusr4_fsdata*) fs_chain_find(waypointp->fs, FS_LOWRANCEUSR4); if (fs && fs->uid_unit == uid_unit && @@ -495,13 +495,13 @@ lowranceusr4_find_waypt(uint uid_unit, int uid_seq_low, int uid_seq_high) return nullptr; } -static Waypoint* +static const Waypoint* lowranceusr4_find_global_waypt(uint id1, uint id2, uint id3, uint id4) { lowranceusr4_fsdata* fs = nullptr; // Iterate with waypt_disp_all? - foreach (Waypoint* waypointp, *global_waypoint_list) { + for (const Waypoint* waypointp : qAsConst(*global_waypoint_list)) { fs = (lowranceusr4_fsdata*) fs_chain_find(waypointp->fs, FS_LOWRANCEUSR4); if (fs && fs->UUID1 == id1 && @@ -610,7 +610,7 @@ static int lowranceusr_common_find_icon_number_from_desc(const QString& desc, co * Also return the icon number for descriptions of "icon-" * followed by a numeric icon number. */ - int n = desc.mid(desc.startsWith("icon-") ? 5 : 0).toInt(); + int n = desc.midRef(desc.startsWith("icon-") ? 5 : 0).toInt(); if (n) { return n; } @@ -1236,7 +1236,7 @@ lowranceusr4_parse_route() uint uid_unit = gbfgetint32(file_in); uint uid_seq_low = gbfgetint32(file_in); uint uid_seq_high = gbfgetint32(file_in); - Waypoint* wpt_tmp = lowranceusr4_find_waypt(uid_unit, uid_seq_low, uid_seq_high); + const Waypoint* wpt_tmp = lowranceusr4_find_waypt(uid_unit, uid_seq_low, uid_seq_high); if (wpt_tmp) { if (global_opts.debug_level >= 2) { printf(MYNAME " parse_route: added leg #%d routepoint %s (%+.10f, %+.10f)\n", @@ -1252,7 +1252,7 @@ lowranceusr4_parse_route() UUID2 = gbfgetint32(file_in); UUID3 = gbfgetint32(file_in); UUID4 = gbfgetint32(file_in); - Waypoint* wpt_tmp = lowranceusr4_find_global_waypt(UUID1, UUID2, UUID3, UUID4); + const Waypoint* wpt_tmp = lowranceusr4_find_global_waypt(UUID1, UUID2, UUID3, UUID4); if (wpt_tmp) { if (global_opts.debug_level >= 2) { printf(MYNAME " parse_route: added leg #%d routepoint %s (%+.10f, %+.10f)\n", diff --git a/netstumbler.cc b/netstumbler.cc index c011c74a6..1c4679f8e 100644 --- a/netstumbler.cc +++ b/netstumbler.cc @@ -43,7 +43,7 @@ static char* sneicon = nullptr; static char* snmac = nullptr; static int macstumbler; -static void fix_netstumbler_dupes(); +static void fix_netstumbler_dupes(const WaypointList*); #define MYNAME "NETSTUMBLER" @@ -98,6 +98,7 @@ data_read() int speed = 0, channel = 0; struct tm tm; int line = 0; + WaypointList tmp_waypt_list; memset(&tm, 0, sizeof(tm)); @@ -251,9 +252,14 @@ data_read() wpt_tmp->latitude = lat; wpt_tmp->SetCreationTime(mktime(&tm)); - waypt_add(wpt_tmp); + tmp_waypt_list.waypt_add(wpt_tmp); + } + + fix_netstumbler_dupes(&tmp_waypt_list); + + for (Waypoint* wpt : tmp_waypt_list) { + waypt_add(wpt); } - fix_netstumbler_dupes(); } struct htable_t { @@ -296,19 +302,16 @@ compare(const void* a, const void* b) static void -fix_netstumbler_dupes() +fix_netstumbler_dupes(const WaypointList* waypt_list) { - int ct = waypt_count(), serial = 0; + int ct = waypt_list->count(), serial = 0; unsigned long last_crc; htable_t* htable = (htable_t*) xmalloc(ct * sizeof *htable); htable_t* bh = htable; int i = 0; - // Why, oh, why is this format running over the entire waypoint list and - // modifying it? This seems wrong. - extern WaypointList* global_waypoint_list; - foreach(Waypoint* waypointp, *global_waypoint_list) { + for (Waypoint* waypointp : *waypt_list) { bh->wpt = waypointp; QString snptr = bh->wpt->shortname; QString tmp_sn = snptr.toLower(); diff --git a/tomtom.cc b/tomtom.cc index 9166dabab..33ecc6a76 100644 --- a/tomtom.cc +++ b/tomtom.cc @@ -251,7 +251,7 @@ data_read() struct hdr { - Waypoint* wpt; + const Waypoint* wpt; }; static int compare_lon(const void* a, const void* b); @@ -377,7 +377,7 @@ compute_blocks(struct hdr* start, int count, for (int i = 0; i < count; i++) { newblock->size += 4 * 3 + 1; /* wpt const part 3 longs, 1 char */ - Waypoint* wpt = start[i].wpt; + const Waypoint* wpt = start[i].wpt; newblock->size += wpt->description.length() + 1; } } else { @@ -438,7 +438,7 @@ data_write() bh = htable; // Iterate with waypt_disp_all? - foreach(Waypoint* waypointp, *global_waypoint_list) { + for (const Waypoint* waypointp : qAsConst(*global_waypoint_list)) { bh->wpt = waypointp; if (waypointp->longitude > maxlon) { maxlon = waypointp->longitude; -- 2.30.2